Add Dracula semantic colors (yellow names, red fails, state-colored connect)#41
Merged
Conversation
Add a protocolColor theme attr for the profile-row protocol/type label. The base themes default it to ?attr/accentOrTextSecondary (preserving the current accent-colored behavior for all themes), and the Dracula day/night styles override it with Dracula green (#50fa7b). getProtocolColor() now reads R.attr.protocolColor instead of accentOrTextSecondary directly. Verified on device: Dracula, Black (deepest attr chain) and Pink_SSR all start with no crash and no attribute-resolution failures.
Dracula forces night mode on, so the active styles at runtime are the values-night Theme.SagerNet.Dracula / Dialog.Dracula variants. They redeclare the Dracula colors but were missing the protocolColor override, so the label fell back to the base ?accentOrTextSecondary (pink) and the green never applied. Add protocolColor=color_dracula_green to both night styles. Verified in the compiled APK via aapt2: night Dracula styles now resolve protocolColor to @color/color_dracula_green (#ff50fa7b). Fixes the issue Greptile flagged on PR #39.
…onnect Extend the Dracula theme's semantic palette via theme attrs (default to the prior colors for all other themes, so they are unchanged): - profileNameColor: profile name label -> Dracula yellow (#f1fa8c) - testFailColor: connection-test failures -> Dracula red (#ff5555) - statusConnectedColor / statusStoppedColor: the bottom StatsBar status text and the connect-button (airplane) icon are tinted green when Connected and red when Stopped; transient Connecting/Stopping states stay neutral. All overrides are duplicated into values-night (Dracula's active path) as well as day. Verified in the compiled APK via aapt2 that the night Dracula style resolves profileNameColor=yellow, statusConnected=green, statusStopped/testFail=red.
📝 WalkthroughWalkthroughIntroduces eleven semantic theme color attributes and three Dracula accent color resources, wires them through all theme variants (base, dialog, Dracula, night, v26, black), and updates UI components to resolve colors from these attributes. ServiceButton tints its FAB icon by service state; StatsBar renders two-tone status text and colored speed metrics; ConfigurationFragment applies theme colors to test results and profile names; RouteFragment and Protocols resolve route and protocol colors from attributes; navigation selectors use the new navSelectedColor attribute. ChangesSemantic color theming system
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/res/values/themes.xml`:
- Around line 669-670: The Dracula dialog style currently only overrides
protocolColor but is missing overrides for profileNameColor,
statusConnectedColor, statusStoppedColor, and testFailColor, causing these
attributes to fall back to base dialog defaults and breaking semantic
consistency. Add item elements for each of these four missing color attributes
in the Dracula dialog style (near the existing protocolColor override),
assigning each an appropriate Dracula color value that maintains visual
consistency with the rest of the Dracula theme palette.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bcb9d0bd-c94a-4316-9f4c-9e18df34147a
📒 Files selected for processing (9)
app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.ktapp/src/main/java/io/nekohasekai/sagernet/widget/ServiceButton.ktapp/src/main/java/io/nekohasekai/sagernet/widget/StatsBar.ktapp/src/main/java/moe/matsuri/nb4a/Protocols.ktapp/src/main/res/values-night/themes.xmlapp/src/main/res/values-v26/themes.xmlapp/src/main/res/values/attrs.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/themes.xml
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Addresses reviewer feedback on PR: - P1: applyStateTint set imageTintList to ?textColorPrimary on every theme, which on non-Dracula light themes painted the connect-FAB icon a dark/wrong color, and cleared the tint to null on transient states. Default statusConnectedColor/statusStoppedColor to ?attr/colorOnPrimary (the FAB's normal on-primary icon color) and use colorOnPrimary for transient states, so non-Dracula themes keep their original appearance. StatsBar status text uses the same fallback (it sits on the colorPrimary bar). - Major/P2: add the four missing semantic overrides (profileName/status/ testFail) to Theme.SagerNet.Dialog.Dracula in both day and night so the Dracula dialog palette stays consistent. Verified in compiled APK: base theme status attrs resolve to ?colorOnPrimary; Dracula resolves to yellow/green/red.
…an proxy, pink nav Per design review of the live theme: - speedTextColor: bottom-bar up/down speed text (incl. 0 B/s) -> Dracula yellow. - fabStoppedColor: connect-FAB icon when Stopped -> Dracula yellow (the red strikethrough plane was nearly invisible on the pink/purple FAB). Status TEXT for stopped/shutting-down stays red via statusStoppedColor. - Two-tone status text: 'Connected,' / 'Success:' lead segment in green, the remainder (hint / handshake detail) in yellow (statusDetailColor). 'Testing…' yellow, 'Shutting down…' red. Falls back to single lead color if a locale string lacks the separator (safe, no crash). - routeProxyColor: Proxy route rule -> Dracula cyan #8be9fd (Dracula's blue). - navSelectedColor: selected nav-drawer item (fill + icon/text) -> Dracula accent (pink) to match the toggle-on color. Black keeps its accent; other themes default to colorPrimary (unchanged). All gated via theme attrs that default to prior colors, so the other ~22 themes are visually unchanged. Verified attr resolution in the compiled APK.
Per design review: switch speedTextColor (up/down + 0 B/s), fabStoppedColor
(stopped airplane icon), and statusDetailColor ('tap to check connection' /
'…handshake took Nms') from yellow to Dracula cyan (#8be9fd). Profile names
stay yellow; protocol=green, connected=green, stopped/fail status text=red,
proxy route=cyan, nav selected=accent. Other themes unchanged.
Add a dedicated statusTestingColor attr so the connection-test 'Testing…' in-progress text is Dracula yellow, separate from statusDetailColor (cyan) used for the 'tap to check connection' hint and the 'handshake took Nms' detail. Defaults to colorOnPrimary for other themes (unchanged).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/io/nekohasekai/sagernet/widget/StatsBar.kt (1)
196-201:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore the normal status after a failed connection test
At Line 200 and Line 201, the catch path sets
"Testing…"again after the request already failed, so the bar can remain in a stale testing state until some later state change. Restore from current service state instead.Suggested fix
onMainDispatcher { isEnabled = true - statusText.setTextColor(context.getColorAttr(R.attr.statusTestingColor)) - setStatus(app.getText(R.string.connection_test_testing)) + changeState(DataStore.serviceState) activity.snackbar( app.getString( R.string.connection_test_error, e.readableMessage )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/io/nekohasekai/sagernet/widget/StatsBar.kt` around lines 196 - 201, In the catch block that handles exceptions during the connection test, the setStatus call is incorrectly resetting the status back to the testing state using R.string.connection_test_testing, which leaves the bar in a stale state after the test fails. Instead of calling setStatus with the testing string resource, restore the status from the current service state to properly reflect the actual connection condition after the failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/main/java/io/nekohasekai/sagernet/widget/StatsBar.kt`:
- Around line 196-201: In the catch block that handles exceptions during the
connection test, the setStatus call is incorrectly resetting the status back to
the testing state using R.string.connection_test_testing, which leaves the bar
in a stale state after the test fails. Instead of calling setStatus with the
testing string resource, restore the status from the current service state to
properly reflect the actual connection condition after the failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2aa6d40d-081f-40c0-a9b9-3d2d6d298042
📒 Files selected for processing (5)
app/src/main/java/io/nekohasekai/sagernet/widget/StatsBar.ktapp/src/main/res/values-night/themes.xmlapp/src/main/res/values-v26/themes.xmlapp/src/main/res/values/attrs.xmlapp/src/main/res/values/themes.xml
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/values/attrs.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/res/values-v26/themes.xml
- app/src/main/res/values-night/themes.xml
- app/src/main/res/values/themes.xml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the Dracula theme with a small semantic-color palette, on top of the existing purple/pink/green-protocol-label work. Everything is gated through new theme attributes that default to the previous colors for all other themes, so the other ~22 themes are visually unchanged.
#50fa7bprotocolColor#f1fa8cprofileNameColor#ff5555testFailColor#50fa7bstatusConnectedColor#ff5555statusStoppedColorSemantic intent: purple = structure, pink = accent, green = good/connected, yellow = identity (name), red = failed/stopped.
Behavior
Implementation
attrs.xml; defaults set on baseTheme.SagerNet, its Dialog, and the v26 base so non-Dracula themes resolve to their prior colors.ConfigurationFragmentreadsprofileNameColor/testFailColor;StatsBarandServiceButtonreadstatusConnectedColor/statusStoppedColorbyBaseService.State.Testing
assembleOssDebugbuilds clean; CodeRabbit local review: 0 findings.profileNameColor=yellow,statusConnectedColor=green,statusStoppedColor=testFailColor=red,protocolColor=green.Not for merge yet — pending visual review (incl. live connect/disconnect state colors) on device.
Greptile Summary
Extends the Dracula theme with a semantic color palette (green=connected/good, yellow=identity/testing, red=failed/stopped, cyan=detail) by introducing eleven new theme attributes with safe defaults that leave all other themes visually unchanged.
attrs.xmlwith defaults onTheme.SagerNet,Theme.SagerNet.Dialog, and the v26 base; Dracula day/night and Dialog.Dracula now carry all overrides, closing the gap flagged in the previous review round.ConfigurationFragmentandStatsBarread the new attrs for profile name, ping-fail, connection-state status text, and speed row;ServiceButton.applyStateTint()appliesstatusConnectedColor/fabStoppedColorto the FAB icon withcolorOnPrimaryas the transient/non-Dracula fallback.nav_item_fill.xml,navigation_icon.xml) is indirected through the newnavSelectedColorattr, allowing Dracula to use its accent (pink) while other themes continue to use their primary color.Confidence Score: 5/5
Safe to merge; all changes are additive theme attribute plumbing with conservative defaults that preserve existing appearance for the ~22 non-Dracula themes.
Every new attribute has a defined default in the base theme, so resolution failures are not possible. The only inconsistency found is in StatsBar's connection-test error path, which applies the testing color rather than the failure color — a cosmetic UX mismatch that doesn't affect functionality, data, or other themes.
StatsBar.kt — the catch block in testConnection() could align its color with testFailColor for consistency with the ConfigurationFragment ping-fail path.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[BaseService.State change] --> B[ServiceButton.changeState] A --> C[StatsBar.changeState] B --> D[applyStateTint] D --> |Connected| E[statusConnectedColor\nDracula: green] D --> |Stopped| F[fabStoppedColor\nDracula: cyan] D --> |Connecting/Stopping| G[colorOnPrimary\nall themes: unchanged] C --> |Connected| H[setStatusTwoTone\nstatusConnectedColor + statusDetailColor] C --> |Other states| I[setStatusColorByState] I --> |Stopped/Stopping| K[statusStoppedColor\nDracula: red] I --> |else| L[colorOnPrimary] C --> |updateSpeed| M[speedTextColor\nDracula: cyan] N[StatsBar.testConnection] --> |start| O[statusTestingColor\nDracula: yellow] N --> |success| P[setStatusTwoTone\nstatusConnectedColor + statusDetailColor] N --> |exception catch| Q[statusTestingColor\nDracula: yellow — not testFailColor]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[BaseService.State change] --> B[ServiceButton.changeState] A --> C[StatsBar.changeState] B --> D[applyStateTint] D --> |Connected| E[statusConnectedColor\nDracula: green] D --> |Stopped| F[fabStoppedColor\nDracula: cyan] D --> |Connecting/Stopping| G[colorOnPrimary\nall themes: unchanged] C --> |Connected| H[setStatusTwoTone\nstatusConnectedColor + statusDetailColor] C --> |Other states| I[setStatusColorByState] I --> |Stopped/Stopping| K[statusStoppedColor\nDracula: red] I --> |else| L[colorOnPrimary] C --> |updateSpeed| M[speedTextColor\nDracula: cyan] N[StatsBar.testConnection] --> |start| O[statusTestingColor\nDracula: yellow] N --> |success| P[setStatusTwoTone\nstatusConnectedColor + statusDetailColor] N --> |exception catch| Q[statusTestingColor\nDracula: yellow — not testFailColor]Reviews (5): Last reviewed commit: "Keep Testing... yellow while detail text..." | Re-trigger Greptile